-
Notifications
You must be signed in to change notification settings - Fork 141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #3428 - Ensure calls to ASTParser.createBindings use the correct unit resolver #3429
Conversation
Alright, so this is just something missed in #2560 . Update: Also would it kill GitHub to just show me this .. diff--- old.txt 2025-01-31 15:09:18.545402211 -0500
+++ new.txt 2025-01-31 15:10:31.175767184 -0500
@@ -1,24 +1,25 @@
- public static IBinding[] resolve(
+ static IBinding[] resolve(
final IJavaElement[] elements,
int apiLevel,
- Map compilerOptions,
+ Map<String,String> compilerOptions,
IJavaProject javaProject,
WorkingCopyOwner owner,
int flags,
+ ICompilationUnitResolver unitResolver,
IProgressMonitor monitor) {
final int length = elements.length;
- final HashMap sourceElementPositions = new HashMap(); // a map from ICompilationUnit to int[] (positions in elements)
+ final HashMap<IJavaElement, IntArrayList> sourceElementPositions = new HashMap<>(); // a map from ICompilationUnit to int[] (positions in elements)
int cuNumber = 0;
final HashtableOfObjectToInt binaryElementPositions = new HashtableOfObjectToInt(); // a map from String (binding key) to int (position in elements)
for (int i = 0; i < length; i++) {
IJavaElement element = elements[i];
if (!(element instanceof SourceRefElement))
throw new IllegalStateException(element + " is not part of a compilation unit or class file"); //$NON-NLS-1$
- Object cu = element.getAncestor(IJavaElement.COMPILATION_UNIT);
+ IJavaElement cu = element.getAncestor(IJavaElement.COMPILATION_UNIT);
if (cu != null) {
// source member
- IntArrayList intList = (IntArrayList) sourceElementPositions.get(cu);
+ IntArrayList intList = sourceElementPositions.get(cu);
if (intList == null) {
sourceElementPositions.put(cu, intList = new IntArrayList());
cuNumber++;
@@ -56,7 +57,7 @@
@Override
public void acceptAST(ICompilationUnit source, CompilationUnit ast) {
// TODO (jerome) optimize to visit the AST only once
- IntArrayList intList = (IntArrayList) sourceElementPositions.get(source);
+ IntArrayList intList = sourceElementPositions.get(source);
for (int i = 0; i < intList.length; i++) {
final int index = intList.list[i];
SourceRefElement element = (SourceRefElement) elements[index];
@@ -76,6 +77,6 @@
}
}
Requestor requestor = new Requestor();
- resolve(cus, bindingKeys, requestor, apiLevel, compilerOptions, javaProject, owner, flags, monitor);
+ unitResolver.resolve(cus, bindingKeys, requestor, apiLevel, compilerOptions, javaProject, owner, flags, monitor);
return requestor.bindings;
} |
} finally { | ||
// reset to defaults to allow reuse (and avoid leaking) | ||
initializeDefaults(); | ||
} | ||
} | ||
|
||
|
||
static IBinding[] resolve( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this is API, and CompilationUnitResolver.resolve(..)
was API, should it not have had some kind of javadoc ? I know nearly all the methods in CompilationUnitResolver
don't but most in ASTParser
do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I would just rename the commit title to Ensure calls to ASTParser.createBindings use correct unit resolver
and mention - Fixes #3428
in the body. Also can probably get rid of your extra "Cleanup" and Signed-off-by message.
- Fixes eclipse-jdt#3428 Signed-off-by: Rob Stryker <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine, just updated the commit message. Description + return explanation should be enough, given the parameters are mostly self explanatory (well except maybe flags).
Most entry points to resolving bindings or parsing compilation units have been updated to make use of the recently-added extension point, to ensure that adopters that wish to extend this functionality are able to do so.
Unfortunately, one of the entry points was missed. ASTParser still had code that traversed directly into ECJ code without checking for the proper resolver.
This PR would fix that.
What it does
How to test
Author checklist